Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for watch progress for the session #184

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

actuallyabhi
Copy link

Outcome

This PR is to save the watched video duration for various videos, so that they can resume playing from where they left off.
In the current implementation, I am saving time_stamps in session storage, so these persist only for the current session.
I am expecting it to make persistent in future PRs.

Demo

demo video

@user234683
Copy link
Owner

Since the persistent version would have to be done quite differently, would prefer we just do that first. I wrote out an implementation plan here if you or anyone else would like to give it a shot:
#155 (comment)

@actuallyabhi
Copy link
Author

@user234683 Sorry for the delay. I forgot about this. I would like to implement it. Probably start on this weekend. I'll keep you posted. Thanks!

@CodiumAI-Agent
Copy link

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Key issues to review

Code Robustness
The extraction of video_id from the URL is fragile and may fail if the URL structure changes or includes additional parameters before the video ID. Consider using URLSearchParams for a more robust solution.

Error Handling
There is no error handling for cases where sessionStorage might be disabled or full, which could lead to unhandled exceptions.

Code Duplication
The sessionStorage.getItem(video_id) is called twice, which is inefficient. Consider storing the result in a variable and reusing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants